ci: bump pinned GitHub Actions to their Node 24 majors - #97
Conversation
📝 WalkthroughWalkthroughUpdates GitHub Actions versions across the composite setup action and CI, build, release, diagnostic, notification, and maintenance workflows. Existing commands, artifact handling, and publishing logic remain unchanged. ChangesGitHub Actions upgrades
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Review — LGTM
Mechanical, well-scoped bump of six first-party actions/* dependencies to the major that declares using: node24 in their manifest. CI is fully green across all three runner platforms (ubuntu / windows / macos), which validates every bumped action in this repo on real runners before merge.
Verified each pinned tag (v7/v6/v7/v6/v7/v8 across checkout, setup-node, setup-python, cache, upload-artifact, download-artifact) declares using: node24 in its action.yml, and audited the breaking-change surface against actual call sites — no impact (no artifact-ids:, setup-node package-manager-cache is overridden by the composite's explicit cache: npm, upload-artifact v4→v7 overwrite default flip is safe because artifact names are unique per job/matrix).
Doc drift (non-blocking, please track)
docs/github-actions-workflows.md:243still showsactions/setup-node@v4in the documented composite-action snippet — should be@v7.docs/secrets.md:58references "the defaultactions/checkout@v4auth" — should be@v7.
These are stale by a few PRs already; happy to send a tiny follow-up PR to fix them once this lands.
Follow-up considerations (not blocking this PR)
actions/github-script@v7inmerged-pr-bookkeeping.yml:19is already Node 24 native; latest isv9.0.0. Could ride along in a follow-up.amannn/action-semantic-pull-request@v5inci.yml:49runs theValidate PR title (semantic)check thatAGENTS.mdflags as required for merge; it was on Node 20 last I checked. Worth a follow-up.- Consider adding
.github/dependabot.ymlforgithub-actions— would have caught this bump automatically and would catch the next node-26 cycle as a one-click PR.
| using: composite | ||
| steps: | ||
| - uses: actions/setup-node@v4 | ||
| - uses: actions/setup-node@v7 |
There was a problem hiding this comment.
✓ Correct bump. setup-node@v7 declares using: node24 at this tag. Note that node-version: 22 on line 8 is intentional — the app's package.json#engines pins Node 22.22.1, and the composite action installs Node 22 for npm ci. The Node 24 only affects the runner side (the action's dist/setup/index.js), not the version installed for the app. No drift between runner-side (24) and app-side (22); the two are independent.
Recommend a one-line comment in this file (or in docs/github-actions-workflows.md) explaining the distinction so future maintainers don't "fix" the node-version: 22 to node-version: 24.
| @@ -33,13 +33,13 @@ jobs: | |||
| runs-on: windows-latest | |||
There was a problem hiding this comment.
✓ Bump to actions/checkout@v7 is correct (uses node24 at this tag). The fetch-depth / repository / token inputs are all still supported. Verified that none of this repo's checkout calls use removed v4 inputs.
| @@ -11,23 +11,23 @@ jobs: | |||
| name: Lint | |||
There was a problem hiding this comment.
✓ All four bumps here are actions/checkout@v4 → @v7. These run on ubuntu-latest only. Worth noting: the Lint / Type Check / Test / Build / Validate PR title (semantic) jobs all reference this checkout — bumping to v7 transitively bumps the runner to Node 24 for the entire ci.yml pipeline. Good preventive maintenance before GitHub forces the migration.
| @@ -15,7 +15,7 @@ jobs: | |||
| name: Windows x64 diagnostic bundle | |||
There was a problem hiding this comment.
✓ actions/upload-artifact@v7 and actions/checkout@v7 are the right targets. The diagnostic bundle jobs run on windows-latest, macos-latest, macos-15-intel — bumping these also brings Node 24 to all three platforms' runners. The if-no-files-found, retention-days, and name inputs are still supported.
4b7c39b to
890064d
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
61-64: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable automatic npm caching unless these jobs require it.
actions/setup-node@v7can enable npm caching whenpackage.jsondeclares npm as the package manager, even whencacheis omitted. The docs job explicitly avoids dependency installation, so this cache is unnecessary there. In low-trust workflows, automatic cache restore and save can create cache-poisoning risk.Verify
package.jsonand the workflow triggers. Setpackage-manager-cache: falsefor these steps unless the cache is intentional. GitHub documents this automatic behavior and recommends disabling it when caching is not required for secure operation. (github.com)Proposed configuration change
- uses: actions/setup-node@v7 with: node-version: 22 + package-manager-cache: falseApply the same input to the
.nvmrc-based setup steps.Also applies to: 172-174, 213-215
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 61 - 64, Disable automatic npm caching on every actions/setup-node@v7 step in the workflow, including the .nvmrc-based setup steps referenced by the comment, by explicitly setting package-manager-cache to false. Preserve the existing Node version and setup behavior.Source: Linters/SAST tools
.github/workflows/nix-check.yml (1)
45-45: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable persisted checkout credentials.
actions/checkout@v7stores the checkout token by default, but this job only runsnix runand prints values. Setpersist-credentials: falseunless a later step needs authenticated Git access.Proposed fix
- - uses: actions/checkout@v7 + - uses: actions/checkout@v7 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/nix-check.yml at line 45, Update the actions/checkout step to set persist-credentials to false, preserving the existing workflow since no later step requires authenticated Git access.Sources: MCP tools, Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 61-64: Disable automatic npm caching on every
actions/setup-node@v7 step in the workflow, including the .nvmrc-based setup
steps referenced by the comment, by explicitly setting package-manager-cache to
false. Preserve the existing Node version and setup behavior.
In @.github/workflows/nix-check.yml:
- Line 45: Update the actions/checkout step to set persist-credentials to false,
preserving the existing workflow since no later step requires authenticated Git
access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: daef0565-e840-4264-b1ff-27247fc71be1
📒 Files selected for processing (13)
.github/actions/setup/action.yml.github/workflows/build-whisper-stt.yml.github/workflows/build.yml.github/workflows/bump-nix-package.yml.github/workflows/ci.yml.github/workflows/diagnostic-artifact.yml.github/workflows/discord-pr-notify.yml.github/workflows/discord-roadmap-sync.yml.github/workflows/discord-weekly-leaderboard.yml.github/workflows/nix-check.yml.github/workflows/prerelease.yml.github/workflows/promote.yml.github/workflows/update-homebrew-cask.yml
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/workflows/update-homebrew-cask.yml
- .github/workflows/bump-nix-package.yml
- .github/actions/setup/action.yml
- .github/workflows/prerelease.yml
- .github/workflows/promote.yml
Every workflow run was logging:
Node.js 20 is deprecated. The following actions target Node.js 20 but
are being forced to run on Node.js 24: actions/checkout@v4, actions/setup-node@v4 ...
GitHub Actions is dropping the Node 20 shim; these actions still declared
`using: node20` at the pinned majors, so runners were silently forcing them
onto Node 24 anyway. Bump to the majors that declare `using: node24` natively
(verified against each repo's action.yml at the release tag, not the
changelog):
actions/checkout v4 -> v7
actions/setup-node v4 -> v7
actions/setup-python v5 -> v6
actions/cache v4 -> v6
actions/upload-artifact v4 -> v7
actions/download-artifact v4 -> v8
actions/github-script v7 -> v8
amannn/action-semantic-pull-request v5 -> v6
Checked every intermediate major's release notes against how this repo
actually calls these actions:
- download-artifact v5 changed the output path for single downloads by
artifact-id; all four call sites here download by `name:`, so unaffected.
- download-artifact v8 now defaults `digest-mismatch` to `error` rather than
a warning. Kept: a corrupt installer should fail the release job rather
than be published silently.
- setup-node v5 auto-enables npm caching when package.json declares
`packageManager` (it does: npm@10.9.4). The composite action already set
`cache: npm`; the three direct call sites in ci.yml gain caching they did
not have. package-lock.json is at the repo root, so detection resolves
rather than erroring.
- checkout v7 refuses to check out fork PR code under pull_request_target /
workflow_run. The guard only runs when the step overrides `repository:` or
`ref:` — the three pull_request_target workflows here do a bare default
checkout, and the two that do override a ref run on `release` /
`workflow_dispatch`. No call site trips it.
- github-script v8 is a pure runtime bump. Stopped there rather than v9,
which injects `getOctokit` as a script parameter and drops
`require('@actions/github')`; merged-pr-bookkeeping.yml uses neither, so
v9 would work too, but v8 leaves the script context identical.
- checkout v5+ and github-script v8 need runner >= v2.327.1. Every job here
runs on a GitHub-hosted runner, so that is satisfied.
- Everything else across these ranges is the Node 24 runtime bump, an ESM
migration transparent to callers, or new opt-in inputs nothing here uses.
Verified empirically rather than by inspection alone: ci.yml on main emits
the deprecation warning on every job, and the same run on this branch emits
none.
Two node20 references are deliberately left:
- ilammy/msvc-dev-cmd@v1 has no newer major published upstream.
- docs.yml is SHA-pinned rather than tag-pinned, and bumping it means
upload-pages-artifact v4+, which stopped including dotfiles and would drop
the `.nojekyll` Docusaurus writes into website/build. That needs
`include-hidden-files: true` and its own verification against the Pages
deploy, which a PR run cannot exercise — the deploy job only runs on push
to main.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3084809 to
66f0cf2
Compare
The tag-pinned actions moved to their node24 majors in #97, but docs.yml pins by SHA and was left out of that sweep, so it still logs: Node.js 20 is deprecated. The following actions target Node.js 20 but are being forced to run on Node.js 24 ... Bump all four, keeping the SHA pinning this file deliberately uses: actions/checkout v4.1.1 -> v7.0.1 actions/setup-node v4.0.3 -> v7.0.0 actions/upload-pages-artifact v3.0.1 -> v5.0.0 actions/deploy-pages v4.0.5 -> v5.0.0 upload-pages-artifact is a composite; what matters for the warning is the actions/upload-artifact@v7.0.0 it now calls internally, which is node24. The one real behavior change is upload-pages-artifact v4, which stopped bundling dotfiles. Docusaurus writes a `.nojekyll` into website/build, so that would have silently dropped it. The artifact-based Pages deploy never runs Jekyll and would most likely not care, but that is not worth verifying on the live site: `include-hidden-files: true` (added in v5) restores exactly what v3 uploaded. `.git` and `.github` stay excluded regardless. Changing this file matches the workflow's own `paths` filter, so the build and upload steps are exercised by this PR. The deploy job only runs on push to main and stays unvalidated until merge — it is a pure runtime bump, with no input or output changes between v4.0.5 and v5.0.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Problem
Every workflow run logs:
GitHub is dropping the Node 20 shim. These actions still declare
using: node20at the pinned majors in this repo, so runners are already silently forcing them onto Node 24 — a warning today, a hard break later.Fix
Bump each action to the major that declares
using: node24natively (verified directly against each repo'saction.ymlat the release tag, not against changelog claims):actions/checkoutactions/setup-nodeactions/setup-pythonactions/cacheactions/upload-artifactactions/download-artifactamannn/action-semantic-pull-requestactions/github-scriptApplied across all 15 workflow/composite-action files that pin them by version tag — 47 call sites. Rebased onto current
main, so it also coversannounce-release.yml,build-whisper-stt.ymlandnix-check.yml, which landed after this branch was first cut.Also verified empirically rather than by inspection alone:
ci.ymlonmainemits the deprecation warning on every job, and the same run on this branch emits none.Breaking changes checked
Read the release notes for every intermediate major and checked each against how this repo actually calls these actions:
name:, so unaffected.digest-mismatchtoerrorinstead of logging a warning. Kept as-is: a corrupt installer should fail the release job rather than be published silently.package.jsondeclarespackageManager, and this repo declaresnpm@10.9.4. The composite action already setscache: npmexplicitly; the three directsetup-nodesteps inci.ymlgain caching they did not have before.package-lock.jsonis at the repo root, so detection resolves rather than erroring.>= v2.327.1. Every job here runs on a GitHub-hosted runner, so that is satisfied automatically; there are no self-hosted runners in this repo.pull_request_target/workflow_run. The guard only runs when the step overridesrepository:orref:(src/input-helper.tsexempts the default self-checkout). The threepull_request_targetworkflows here do a bare default checkout, and the two workflows that do override a ref (update-homebrew-cask,bump-nix-package) run onrelease/workflow_dispatch, where the guard early-returns. No call site trips it.typesandrequireScope, both unchanged.v7.1.0...v8.0.0is the Node 24 change plus a README). Stopped at v8 rather than v9 deliberately: v9 injectsgetOctokitas a script parameter and dropsrequire('@actions/github'). The 252-line script inmerged-pr-bookkeeping.ymluses neither, so v9 would also work, but v8 leaves the script context byte-for-byte identical.archive: false) that nothing here uses.Deliberately not in this PR
ilammy/msvc-dev-cmd@v1is stillnode20and upstream has published no newer major, so one deprecation line survives until it does..github/workflows/docs.ymlis SHA-pinned rather than tag-pinned, and its four actions are node20 too. It is left alone becauseupload-pages-artifactv4 stopped including dotfiles, which would drop the.nojekyllDocusaurus emits intowebsite/build. Fixing it meansv5plusinclude-hidden-files: true, and it touches the production Pages deploy — the deploy job only runs on push tomain, so a PR run cannot validate it. Worth its own PR.🤖 Generated with Claude Code